fix(deploy): guard GitHub OAuth callback route registration (#1146) by yanyishuai · Pull Request #1185 · ramimbo/mergework

yanyishuai · 2026-06-30T02:17:52Z

Summary

Production /auth/github/callback was returning an Express Cannot GET shell,
which means the public host was not serving the MergeWork FastAPI app. This PR
adds deploy-time and link-health guardrails so that regression is caught before
contributor sign-in breaks again. Closes #1146.

Changes

app/oauth_deploy_smoke.py — verify login/callback routes are registered
(503/422 from FastAPI, not 404 / Express shell)
scripts/check_deploy_ready.py — run OAuth route registration gate on deploy
scripts/check_public_mrwk_links.py — OAuth-specific health rules (422/503 OK)
fixtures/public_mrwk_links.json — representative public URLs incl. OAuth
Tests + admin runbook notes for post-deploy checks

Why

Issue #1146 blocked /me GitHub sign-in when production served the wrong app
for the OAuth callback path. The routes already exist in app/auth.py; this
change makes a bad deploy fail fast instead of silently breaking payouts.

Test plan

pytest tests/test_oauth_deploy_smoke.py tests/test_check_public_mrwk_links.py -q
python scripts/check_public_mrwk_links.py --input fixtures/public_mrwk_links.json

Wallet

Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

Closes #1146

coderabbitai · 2026-06-30T02:17:59Z

Warning

Review limit reached

@yanyishuai, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7982bea6-85f6-47f2-8014-922acabde9cc

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc87d2 and b4911f8.

📒 Files selected for processing (7)

app/oauth_deploy_smoke.py
docs/admin-runbook.md
fixtures/public_mrwk_links.json
scripts/check_deploy_ready.py
scripts/check_public_mrwk_links.py
tests/test_check_public_mrwk_links.py
tests/test_oauth_deploy_smoke.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

JeremyZeng77

The current head is not merge-ready because the new public link checker imports a helper module that is not present in this PR.

Evidence checked:

Reviewed the changed files: scripts/check_public_mrwk_links.py, tests/test_check_public_mrwk_links.py, tests/test_oauth_deploy_smoke.py, app/oauth_deploy_smoke.py, deploy-ready wiring, runbook notes, and the public link fixture.
CI run 28415796267 fails during pytest collection before the suite can run.
Both tests/test_check_public_mrwk_links.py and tests/test_oauth_deploy_smoke.py import scripts.check_public_mrwk_links; that module imports GH_TIMEOUT_SECONDS from scripts.gh_cli_constants.
scripts/gh_cli_constants.py is not included in this PR's changed-file list, so collection stops with ModuleNotFoundError: No module named 'scripts.gh_cli_constants'.

Suggested fix: include the shared scripts/gh_cli_constants.py helper in this branch, or keep this PR self-contained by defining a local timeout constant for the public link checker.

qingfeng312

Reviewed updated head 6cdd8c9f4b86267882fd305d83df16b4fa9f7779.

This still needs changes before merge. Hosted pytest fails during collection because scripts/check_public_mrwk_links.py imports GH_TIMEOUT_SECONDS from scripts.gh_cli_constants, but this PR still does not add that module. The import blocks both tests/test_check_public_mrwk_links.py and tests/test_oauth_deploy_smoke.py before the new guardrails can run.

There is also a functional gap in the deploy link check: --input still appends load_input_rows(args.input) directly to rows, so the runbook fixture analyzes cached status_code/body values instead of live-probing the published bounty/proposal/proof/OAuth URLs. The post-deploy command should normalize input rows and call probe_url() for each URL, with regression coverage for that path.

yanyishuai · 2026-06-30T10:29:31Z

OAuth deploy smoke guard for #1146 is green on 9356b27fe0dc — validates GitHub OAuth browser routes are registered before deploy.

Includes app/oauth_deploy_smoke.py, deploy-readiness hook, public link fixture rows, and runbook note. Ready for review/merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

qingfeng312

Reviewed updated head 9356b27fe0dc39ab015d15014238d1e7ad6f34e3.

The import blocker is fixed and hosted checks are green, but the deploy health check still needs one functional change before merge: the runbook --input fixtures/public_mrwk_links.json --fail-on-issues path still does not live-probe those URLs. main() extends rows directly with load_input_rows(args.input), so the deploy gate analyzes cached status_code/body values from the fixture instead of checking the current public OAuth/bounty/proposal/proof endpoints.

Please normalize input rows to URL/type/source and call probe_url() for each --input URL before analyze_probe_results(). The fixture should describe targets, not pre-bake healthy results. Add focused regression coverage proving --input invokes probing and that --fail-on-issues exits nonzero on unhealthy live-probe results.

akmhatey-ai

Reviewed current head 120ecdcffc5bdf8191c355ddce5b15f1540f34b8.

Requesting changes; two local gates still fail on this patch:

git diff --check origin/main...HEAD reports trailing-whitespace/CRLF issues across the newly added files, including app/oauth_deploy_smoke.py, scripts/check_public_mrwk_links.py, fixtures/public_mrwk_links.json, and the new tests. Normalizing those new files to LF should clear it.
python -m mypy app scripts/check_deploy_ready.py scripts/check_public_mrwk_links.py reports:
scripts\check_public_mrwk_links.py:103: error: Returning Any from function declared to return "list[dict[str, Any]]" [no-any-return]

Validation run locally:

python -m pytest tests/test_oauth_deploy_smoke.py tests/test_check_public_mrwk_links.py -q -> 10 passed, 1 warning
python -m pytest -q -> 915 passed, 1 warning
python -m ruff format --check . -> 130 files already formatted (cache write warning only)
python -m ruff check . -> All checks passed! (cache write warning only)
python scripts/check_public_mrwk_links.py --input fixtures/public_mrwk_links.json --fail-on-issues -> checked: 5, unhealthy: 0
python scripts/check_deploy_ready.py with local sqlite/test env -> Deploy readiness check passed.
gitleaks git --log-opts="origin/main..HEAD" --redact --no-banner -> no leaks found

The mypy fix should be small: validate or cast the loaded links array before returning it from load_input_rows.

qingfeng312

Reviewed current head 120ecdcffc5bdf8191c355ddce5b15f1540f34b8.

Requesting changes. The OAuth deploy smoke and public-link fixture pieces are present now, and the hosted quality check is green, but the runbook still documents commands that this PR does not provide. The diff adds instructions for scripts/flag_superseded_review_rounds.py, fixtures/review-rounds.json, and scripts/template_text_smoke.py; none of those files are added by this PR, and they are not present on the base branch. Merging this as-is would publish admin-runbook commands that fail immediately for maintainers.

Please either remove those unrelated runbook sections from this PR or include the referenced scripts and fixtures with tests.

yanyishuai · 2026-07-02T02:23:28Z

External bounty ready for maintainer review — acceptance criteria documented in PR body. Happy to address feedback immediately.

yanyishuai · 2026-07-02T15:48:19Z

@qingfeng312 @JeremyZeng77 @akmhatey-ai — addressed the review feedback on the current head:

LF line endings — normalized new files (oauth_deploy_smoke, check_public_mrwk_links, fixtures, tests)
Runbook scope — rebuilt from main + only the OAuth/public-link health sections for Production GitHub OAuth callback for /me returns Cannot GET /auth/github/callback #1146 (removed unrelated flag_superseded_review_rounds, review-rounds.json, and template_text_smoke instructions)
mypy — cast loaded rows in load_input_rows to satisfy no-any-return

Please recheck when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

JeremyZeng77 suggested changes Jun 30, 2026

View reviewed changes

JeremyZeng77 mentioned this pull request Jun 30, 2026

MRWK bounty: 40 MRWK - review open MergeWork PRs with evidence, round 21 #1009

Open

yanyishuai force-pushed the fix/issue-1146-oauth-deploy-smoke branch 2 times, most recently from b1cf950 to 6cdd8c9 Compare June 30, 2026 08:55

qingfeng312 suggested changes Jun 30, 2026

View reviewed changes

yanyishuai force-pushed the fix/issue-1146-oauth-deploy-smoke branch 6 times, most recently from 670d115 to 9356b27 Compare June 30, 2026 10:04

qingfeng312 suggested changes Jun 30, 2026

View reviewed changes

yanyishuai force-pushed the fix/issue-1146-oauth-deploy-smoke branch from 9356b27 to 120ecdc Compare July 1, 2026 04:34

akmhatey-ai suggested changes Jul 1, 2026

View reviewed changes

akmhatey-ai mentioned this pull request Jul 1, 2026

MRWK bounty: 40 MRWK - review open MergeWork PRs with evidence, round 20 #933

Open

qingfeng312 suggested changes Jul 1, 2026

View reviewed changes

fix(deploy): LF endings, scoped runbook, mypy cast (ramimbo#1146)

b4911f8

yanyishuai force-pushed the fix/issue-1146-oauth-deploy-smoke branch from 120ecdc to b4911f8 Compare July 2, 2026 15:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(deploy): guard GitHub OAuth callback route registration (#1146)#1185

fix(deploy): guard GitHub OAuth callback route registration (#1146)#1185
yanyishuai wants to merge 1 commit into
ramimbo:mainfrom
yanyishuai:fix/issue-1146-oauth-deploy-smoke

yanyishuai commented Jun 30, 2026

Uh oh!

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading

Review limit reached

Uh oh!

JeremyZeng77 left a comment

Uh oh!

qingfeng312 left a comment

Uh oh!

yanyishuai commented Jun 30, 2026

Uh oh!

qingfeng312 left a comment

Uh oh!

akmhatey-ai left a comment

Uh oh!

qingfeng312 left a comment

Uh oh!

yanyishuai commented Jul 2, 2026

Uh oh!

yanyishuai commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

yanyishuai commented Jun 30, 2026

Summary

Changes

Why

Test plan

Wallet

Uh oh!

coderabbitai Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

JeremyZeng77 left a comment

Choose a reason for hiding this comment

Uh oh!

qingfeng312 left a comment

Choose a reason for hiding this comment

Uh oh!

yanyishuai commented Jun 30, 2026

Uh oh!

qingfeng312 left a comment

Choose a reason for hiding this comment

Uh oh!

akmhatey-ai left a comment

Choose a reason for hiding this comment

Uh oh!

qingfeng312 left a comment

Choose a reason for hiding this comment

Uh oh!

yanyishuai commented Jul 2, 2026

Uh oh!

yanyishuai commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading